Skip to content

Adding suffix 'field' to classes inheriting IndexBase - #296

Closed
Aksha1812 wants to merge 2 commits into
valkey-io:mainfrom
Aksha1812:fulltext-dev-aksha6
Closed

Aksha1812 wants to merge 2 commits into
valkey-io:mainfrom
Aksha1812:fulltext-dev-aksha6

Conversation

@Aksha1812

Copy link
Copy Markdown
Collaborator

Proposal to update nomenclature : Classes inheriting IndexBase handle a single field/attribute. This is not apparent from the name of the classes inheriting IndexBase. e.g Tag, Text etc.. . proposing to add 'Field' suffix to these classes.

 IndexBase
├── Tag
├── Text  
├── Numeric
└── VectorBase
    ├── VectorFlat
    └── VectorHNSW

changes to

IndexBase
├── TagField
├── TextField  
├── NumericField
└── VectorBaseField
    ├── VectorFlatField
    └── VectorHNSWField

I, Aksha Thakkar <thaakb@amazon.com>, hereby add my Signed-off-by to this commit: 7c412ba

Signed-off-by: Aksha Thakkar <thaakb@amazon.com>
@yairgott

Copy link
Copy Markdown
Collaborator

A couple of points:

  1. The terminology that we're using is Attribute rather than Field to generalize both JSON and Hash. I believe that we're using this terminology in our public documentations (if not, we should).
  2. I think that the it makes sense to also include the following changes:
    2.1. IndexBase -> AttributeBase , note that it's already using the namespace valkey_search::indexes
    2.2. IndexerType , kIndexerTypeByStr, etc -> AttributeType, ...

WDYT?

@allenss-amazon

Copy link
Copy Markdown
Member

Well, I'm 100% in favor of consistency. The Redis documents also use the term Attribute in many places. While I personally find field more usable than attribute, IDC which we end up with as long as it's consistently applied.

@Aksha1812

Aksha1812 commented Aug 20, 2025 •

Copy link
Copy Markdown
Collaborator Author
  1. I believe 'Field' is used more in this context as i have seen in Valkey code https://github.com/search?q=repo%3Avalkey-io%2Fvalkey+attribute&type=code vs https://github.com/search?q=repo%3Avalkey-io%2Fvalkey+field&type=code. I didn't find Attribute being used in the documentation as well. But in valkey-search I understand that we have been using 'Attribute' in this context. lmk if i am wrong. Keeping that in mind are you saying it should rather be called 'TagAttribute' or 'NumericAttribute' rather than using 'TagField' or 'NumericField' to be in line with terminology currently being used.

  2. Agreed will add these changes as well.

@yairgott

Copy link
Copy Markdown
Collaborator

Keeping that in mind are you saying it should rather be called 'TagAttribute' or 'NumericAttribute' rather than using 'TagField' or 'NumericField' to be in line with terminology currently being used.

Correct.

  1. Agreed will add these changes as well.

SG!

@allenss-amazon

Copy link
Copy Markdown
Member

Did we abandon this? If so, let's close it

@Aksha1812

Copy link
Copy Markdown
Collaborator Author

Did we abandon this? If so, let's close it

No haven't abandoned this, just caught up with some fulltext work currently, will update pr soon, once have some extra time

@allenss-amazon

Copy link
Copy Markdown
Member

Update or close please.

@allenss-amazon

Copy link
Copy Markdown
Member

closing. stale

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants